Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linker API Changes #88

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Linker API Changes #88

merged 2 commits into from
Sep 25, 2024

Conversation

george-cosma
Copy link
Collaborator

@george-cosma george-cosma commented Sep 23, 2024

Pull Request Overview

This pull request changes the RuntimeInstance API to be compatible with the #87 linker proposal. The PR follows the proposal highlighted in issue #83, more precisely this comment. This was made to avoid the multitude of functions that would be create when introducing the idea of "modules" and "module_index"

Formatting

  • Ran cargo fmt
  • Ran cargo check
  • Ran cargo build
  • Ran cargo doc
  • Ran nix fmt

Github Issue

This pull request helps close #83

@george-cosma
Copy link
Collaborator Author

I recommend looking at the first commit and the last. The one in the middle only changes the tests.

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 78.15126% with 26 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/execution/mod.rs 81.73% 13 Missing and 6 partials ⚠️
src/execution/function_ref.rs 53.33% 7 Missing ⚠️
Files with missing lines Coverage Δ
src/execution/function_ref.rs 53.33% <53.33%> (ø)
src/execution/mod.rs 91.22% <81.73%> (-5.17%) ⬇️

@george-cosma george-cosma mentioned this pull request Sep 24, 2024
5 tasks
@george-cosma george-cosma force-pushed the dev/api_changes branch 2 times, most recently from fda3926 to ad23b4c Compare September 24, 2024 10:16
Copy link
Collaborator

@florianhartung florianhartung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/execution/function_ref.rs Show resolved Hide resolved
src/execution/mod.rs Outdated Show resolved Hide resolved
src/execution/mod.rs Outdated Show resolved Hide resolved
benches/hook_performance_impact.rs Outdated Show resolved Hide resolved
@george-cosma george-cosma force-pushed the dev/api_changes branch 5 times, most recently from 47111bf to 8c3b877 Compare September 25, 2024 09:58
Copy link
Collaborator

@nerodesu017 nerodesu017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nerodesu017 nerodesu017 added this pull request to the merge queue Sep 25, 2024
Merged via the queue into main with commit 09c3435 Sep 25, 2024
12 checks passed
@nerodesu017 nerodesu017 deleted the dev/api_changes branch September 25, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The never-ending Linker problem
3 participants